-
Notifications
You must be signed in to change notification settings - Fork 11
Refactor authoring section to include byo chunks for contexts #35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor authoring section to include byo chunks for contexts #35
Conversation
| " chunks_jsonl_path = contribution[\"dir\"] / CHUNKING_DIR / \"chunks.jsonl\"\n", | ||
| " authoring_path = contribution[\"dir\"] / AUTHORING_DIR\n", | ||
| "\n", | ||
| " selected_chunks_path = get_random_chunks(chunks_jsonl_path,\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a bit misleading to me because I would expect a function called get_random_chunks to return chunks in some sort of data structure, but it actually saves to a nonobvious path and returns the path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching this, I changed the name of the function to create_random_chunks_jsonl().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think it's misleading - it's not creating chunks is it? save_chunk_selection?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this suggestion a lot, I ended up with save_random_chunk_selection. I like having random in the function name so users understand how the chunks are being selected.
| "nbconvert_exporter": "python", | ||
| "pygments_lexer": "ipython3", | ||
| "version": "3.11.13" | ||
| "version": "3.12.10" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We keep taking turns overwriting this in various PRs. I think we still have the requirement for <= 3.11 for some pieces, right?
|
Sounds like we need to implement some constraints checking in our notebook
CI.
…On Thu, Jun 26, 2025, 11:40 AM Anastas Stoyanovsky ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In notebooks/instructlab-knowledge/instructlab-knowledge.ipynb
<#35 (comment)>:
> @@ -862,7 +913,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
- "version": "3.11.13"
+ "version": "3.12.10"
We keep taking turns overwriting this in various PRs. I think we still
have the requirement for <= 3.11 for some pieces, right?
—
Reply to this email directly, view it on GitHub
<#35 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACNH3CHVIQAU4J6WMKJ56IL3FQIAJAVCNFSM6AAAAACAEVNZSCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDSNRSGU4TEMZSHA>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
1e9e231 to
b14e1a5
Compare
Enable users to bring their own chunks as contexts for question and answer generation. Add quick review of seed examples after questions and answers are genrerated. Signed-off-by: Ali Maredia <[email protected]>
b14e1a5 to
176c4b0
Compare
Enable users to bring their own chunks as contexts for question and answer generation.
Add quick review of seed examples after questions
and answers are genrerated.